Fold/visit tweaks#156130
Conversation
rustbot
added
S-waiting-on-review
|
|
| fn has_type_flags(&self, flags: TypeFlags) -> bool { | ||
| let res = | ||
| self.visit_with(&mut HasTypeFlagsVisitor { flags }) == ControlFlow::Break(FoundFlags); | ||
| res |
There was a problem hiding this comment.
imo comparing with ControlFlow::Break(FoundFlags) is clearer, because it asserts that Break contains FoundFlags
There was a problem hiding this comment.
Even though FoundFlags is a unit type?
There was a problem hiding this comment.
Note that has_vars_bound_at_or_above uses .is_break(). I was fixing the inconsistency and .is_break() seemed nicer.
There was a problem hiding this comment.
Even though FoundFlags is a unit type?
Yes; With == ControlFlow::Break(FoundFlags) it's clearer why Break means that the flags have been found, without looking at surrounding code / type returned by visit_with.
But ultimately this is such a small nitpick...
I always choose a reviewer rather than letting bors randomly assign. Usually I choose somebody who knows the relevant part of the code well. Or if it's an easier PR that doesn't require specific expertise I just pick someone who I haven't picked for a while. |
- Remove comment about blanket implementation of `FallibleTypeFolder` -- it does not exist. (It probably used to.) - Fix some incorrect name references. - Fix minor grammatical errors. - Fix stale comment on `visit_region` -- it was a no-op once, but no longer. LLM disclosure: Claude Code identified these problems with comments when I asked it to review `fold.rs` and `visit.rs`. I verified the correctness of the problems and made the changes by hand.
nnethercote
force-pushed
the
fold-visit-tweaks
branch
from
fd068fd to
dd4ce84
Compare
|
I addressed two of the three comments, and left the |
| fn has_type_flags(&self, flags: TypeFlags) -> bool { | ||
| let res = | ||
| self.visit_with(&mut HasTypeFlagsVisitor { flags }) == ControlFlow::Break(FoundFlags); | ||
| res |
There was a problem hiding this comment.
Even though FoundFlags is a unit type?
Yes; With == ControlFlow::Break(FoundFlags) it's clearer why Break means that the flags have been found, without looking at surrounding code / type returned by visit_with.
But ultimately this is such a small nitpick...
- Introduce `HAS_REGIONS`/`has_regions`, which is true if any regions are present. - Add `fold_clauses` to `Shifter` and `RegionFolder` for consistency. - Simplify `has_type_flags`. - Remove unnecessary local variables in `HasTypeFlagsVisitor` methods. - Use `|=` operator in one place. - Avoid `is_break` in one place for consistency with nearby code. LLM disclosure: Claude Code suggested these changes when I asked it to review `fold.rs` and `visit.rs`. I verified the correctness of the suggestions and made the changes by hand.
nnethercote
force-pushed
the
fold-visit-tweaks
branch
from
dd4ce84 to
803c7ba
Compare
|
I addressed the latest two comments. |
|
@bors r+ rollup |
rust-bors
Bot
added
S-waiting-on-bors
…affleLapkin Fold/visit tweaks Details in individual commits. r? @WaffleLapkin
…uwer Rollup of 7 pull requests Successful merges: - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #156173 (Fewer global node_id_to_def_id lookups) - #155961 (Deny warnings in the test for crates that are available on stable) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156141 (Resolve some cases of #132279 by using the right typing mode in the next solver) - #156202 (llvm: Use correct type for splat mask)
…affleLapkin Fold/visit tweaks Details in individual commits. r? @WaffleLapkin
…uwer Rollup of 7 pull requests Successful merges: - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #149509 (Lint unused pub items in binary crates) - #156173 (Fewer global node_id_to_def_id lookups) - #155961 (Deny warnings in the test for crates that are available on stable) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156202 (llvm: Use correct type for splat mask) Failed merges: - #156236 (resolve: Remove `MacroData`)
…affleLapkin Fold/visit tweaks Details in individual commits. r? @WaffleLapkin
…uwer Rollup of 9 pull requests Successful merges: - #156061 (Support `-Cpanic=unwind` on WASI targets) - #146273 (lint ImproperCTypes: refactor linting architecture (part 2)) - #149509 (Lint unused pub items in binary crates) - #156173 (Fewer global node_id_to_def_id lookups) - #155961 (Deny warnings in the test for crates that are available on stable) - #155981 (Use special DefIds for aliases) - #156130 (Fold/visit tweaks) - #156131 (Metadata macro/query cleanups) - #156202 (llvm: Use correct type for splat mask) Failed merges: - #156236 (resolve: Remove `MacroData`)
3 participants
Details in individual commits.
r? @WaffleLapkin